Skip to content

refactor(cbor): encoder to utils cbor - #4033

Open
RafaelGranza wants to merge 1 commit into
mainfrom
refactor/encoder-to-utils-cbor
Open

refactor(cbor): encoder to utils cbor#4033
RafaelGranza wants to merge 1 commit into
mainfrom
refactor/encoder-to-utils-cbor

Conversation

@RafaelGranza

@RafaelGranza RafaelGranza commented Sep 4, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor

User description

Three things to notice:

  • Rename the package from encoder to utils/cbor/v1
  • Move encoding files from encoder to utils/cbor/v1
  • utils/cbor/registry does not live inside a single version, since it maps extra CBOR tags to special Go objects, and these tags are order generated. It is best to keep them unified for now.

PR Type

Enhancement, Other

Description

  • Rename encoder package to utils/cbor/v1.

  • Move registry files to utils/cbor/registry.

  • Update imports across the entire codebase.

  • Update depguard linter rules in .golangci.yaml.

File Walkthrough
Relevant files
Configuration changes
1 files
.golangci.yaml
update depguard rule for CBOR package                                       
+2/-2     
Refactoring
14 files
cbor.go
rename package to cbor and update definitions                       
+12/-12 
registry.go
update registry to use the new cbor package                           
+2/-2     
hook.go
rename package and SelfEncoder interface                                 
+1/-1     
raw.go
rename package for raw CBOR definitions                                   
+1/-1     
accessors.go
replace encoder references with cbor in accessors               
+21/-21 
class.go
update class definitions to use new cbor package                 
+3/-3     
transaction.go
update transaction encoding to new cbor package                   
+3/-3     
cbor.go
update felt CBOR unmarshalling to new package                       
+2/-2     
partial_cbor.go
implement updated cbor interfaces for partial decoding     
+3/-3     
cbor.go
update db key serializer to new cbor package                         
+2/-2     
cbor.go
update db value serializer to new cbor package                     
+3/-3     
codec.go
update p2p encoding to use new cbor package                           
+3/-3     
juno.go
update CBOR registry import in juno command                           
+1/-1     
migration.go
update db migrations to use new cbor package                         
+10/-10 
Testing
5 files
raw_test.go
remove old raw test file from encoder package                       
+0/-54   
raw_test.go
add raw test file to the new cbor package                               
+54/-0   
golden_test.go
migrate golden tests to new cbor package                                 
+6/-6     
unmarshal_first_test.go
migrate unmarshal first tests to new cbor package               
+6/-6     
on_disk_bytes.json
update test data JSON for cbor.RawMessage                               
+1/-1     
Additional files
56 files
init_test.go +1/-1     
init_test.go +1/-1     
init_test.go +1/-1     
init_test.go +1/-1     
init_test.go +1/-1     
init_test.go +1/-1     
accessors_benchmark_test.go +1/-1     
accessors_test.go +7/-7     
block_transaction_serializer.go +3/-3     
block_transaction_test.go +5/-5     
block_transaction_txhash_test.go +1/-1     
class_test.go +3/-3     
history_test.go +1/-1     
cbor_fastpath_test.go +3/-3     
felt_test.go +3/-3     
slice.go +4/-4     
slice_bench_test.go +6/-6     
slice_test.go +4/-4     
indexed.go +3/-3     
lazy_slice.go +2/-2     
lazy_slice_bench_test.go +2/-2     
lazy_slice_test.go +2/-2     
init_test.go +1/-1     
partial_cbor_test.go +36/-36 
running_event_filter_test.go +3/-3     
accessors.go +3/-3     
accessors_test.go +1/-1     
state_test.go +1/-1     
transaction_test.go +3/-3     
journal.go +7/-7     
nodeset.go +3/-3     
init_test.go +1/-1     
upgrade_test.go +3/-3     
db_utils.go +3/-3     
mempool_test.go +1/-1     
blocktransactions.go +1/-1     
ingestor.go +4/-4     
transaction_layout_test.go +1/-1     
init_pkg_test.go +1/-1     
init_test.go +1/-1     
migration_pkg_test.go +7/-7     
migrator_test.go +1/-1     
migrator_test.go +1/-1     
init_test.go +1/-1     
migration_test.go +1/-1     
init_test.go +1/-1     
accessors_internal_test.go +1/-1     
accessors_test.go +1/-1     
pruner_test.go +1/-1     
retention_test.go +1/-1     
init_test.go +1/-1     
init_test.go +1/-1     
init_test.go +1/-1     
init_test.go +1/-1     
init_test.go +1/-1     
init_test.go +1/-1     

@codecov

codecov Bot commented Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.16129% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.30%. Comparing base (58fcf30) to head (2b5452a).

Files with missing lines Patch % Lines
migration/deprecated/migration.go 77.77% 2 Missing ⚠️
p2p/codec.go 50.00% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #4033   +/-   ##
=======================================
  Coverage   79.30%   79.30%           
=======================================
  Files         465      465           
  Lines       35786    35786           
=======================================
  Hits        28381    28381           
  Misses       7396     7396           
  Partials        9        9

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Base automatically changed from refactor/cbor-encoding-wrapper to main September 5, 2026 15:22
@RafaelGranza
RafaelGranza force-pushed the refactor/encoder-to-utils-cbor branch from 477c91b to 2b5452a Compare September 7, 2026 12:52
RafaelGranza
RafaelGranza commented Sep 7, 2026
View reviewed changes
Comment thread utils/cbor/v1/raw_test.go
Comment on lines +1 to +5
package cbor_test

import (
"testing"

@RafaelGranza RafaelGranza Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was moved from encoder/raw_test.go

@RafaelGranza
RafaelGranza marked this pull request as ready for review September 7, 2026 13:09
@github-actions

github-actions Bot commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Claude finished @RafaelGranza's task in 4m 8s —— View job

Review complete

  • Gather context (PR diff, changed files list)
  • Understand the request (code review per repo custom instructions)
  • Review diff for logic changes vs pure rename/move
  • Check utils/cbor/registry package split rationale
  • Check moved file utils/cbor/v1/raw_test.go
  • Post final summary

This PR is a clean mechanical rename/move (encoder → utils/cbor/v1), with utils/cbor/registry kept separate as explained in the description. I diffed every changed file against origin/main: aside from import path/package-name substitutions (encoder. → cbor.), file contents are byte-for-byte identical, including test assertions, golden vectors, and the registry's tag registration order. go build succeeds across all affected packages (utils/..., core/..., db/..., migration/..., p2p/..., mempool/...), and no stray juno/encoder references remain anywhere in the tree.

No blockers or important issues found.

Nit: .golangci.yaml's new depguard pattern !**/utils/cbor/*/*.go (single wildcard segment) exempts any immediate subdirectory of utils/cbor from the "don't import fxamacker/cbor directly" rule — not just v1. That means utils/cbor/registry/*.go is also silently exempted, even though registry doesn't need direct access to the underlying CBOR library. It's not a functional bug today (registry doesn't import fxcbor), but it slightly widens the enforcement gap versus the original !**/encoder/*.go rule, which allowed exactly one package. Consider a more specific pattern (e.g. !**/utils/cbor/v*/*.go) if you want to preserve the original intent of "only the cbor wrapper package(s) can touch fxamacker directly."

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🏅 Score: 100
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@RafaelGranza